Restore the documented BandedMatrices reexports - #82
Merged
Conversation
Commit f3ec92e ("compat: declare SciMLTesting 2.10 and remove blanket reexports", #81) deleted `@reexport using BandedMatrices` from FastAlmostBandedMatrices. That silently stripped the package's public surface: `using FastAlmostBandedMatrices` no longer brought `BandedMatrix` or `brand` into scope, so the package's own documented construction path stopped working without a separate `using BandedMatrices`. The same commit had to rewrite every docstring example from `brand(...)` to `BandedMatrices.brand(...)` and add `using BandedMatrices` to the README, the docs and nearly every testset -- which is itself the evidence that those names are normal documented use. Following the rule "reexport what is normal documented use, but not whole dependencies", this restores an explicit `export` list instead of the blanket reexport: BandedMatrix, brand, brandn # build the `bands` argument Band, BandRange, band, bandrange # populate it bandwidth, bandwidths, colrange, rowrange # query it BandError # thrown by out-of-band setindex! Everything else BandedMatrices exports stays out, in particular the FillArrays names it reexports in turn (`Fill`, `Ones`, `Zeros`, `Eye`), the `BandedMatrices` module binding itself, and `symrcm`. Evidence for the list: * `AlmostBandedMatrix(bands::BandedMatrix, fill)` is the documented constructor, and every docstring/README/docs example builds `bands` with `brand`; `brandn` is its direct sibling. * `test/core_tests.jl` uses `BandedMatrix`, `brand`, `band(0)` and `@test_throws BandError` against this package's own type, and `test/qa/alloc_tests.jl` uses `brand` -- all of which needed a `using BandedMatrices` added by the stripping commit (in alloc_tests.jl without BandedMatrices even being a dependency of test/qa/Project.toml, which this commit also resolves by dropping that import again). * `bandwidth`/`bandwidths` were the two names the stripping commit itself kept importing, and are how `bandpart(A)` is queried. The docstrings, README and docs examples are restored to their unqualified form, the docs and README now list the reexported surface explicitly, the QA allow-list (`REEXPORTED_API` in test/qa/qa.jl) is restored and kept in sync, and a Core test pins that every approved name is exported, in scope from a bare `using FastAlmostBandedMatrices`, and still identical to the upstream binding -- while the excluded names stay unexported. Restoring previously exported names is not breaking; no version bump here. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rmh6B9eoW3N8oCbuuVPVxJ
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What broke
f3ec92e("compat: declare SciMLTesting 2.10 and remove blanket reexports", #81) deleted@reexport using BandedMatricesfromsrc/FastAlmostBandedMatrices.jl, replacing the bareusing BandedMatriceswithimport BandedMatrices+using BandedMatrices: bandwidth, bandwidths.That silently stripped the package's public surface.
using FastAlmostBandedMatricesno longer putBandedMatrixorbrandin scope, so the package's own documented construction path stopped working on its own. CI never noticed because the same commit patched every call site: it rewrote every docstring example frombrand(Float64, 8, 8, 3, 2)toBandedMatrices.brand(...), addedusing BandedMatricesto the README,docs/src/index.mdand nearly every testset, and addedBandedMatricestotest/Project.toml.That rewrite is the evidence. The documented constructor is
AlmostBandedMatrix(bands::BandedMatrix, fill)and every documented example buildsbandswithbrand, so those names are normal documented use of this package.This is release-blocking: the package is being held out of the release specifically because of this stripped surface.
What this PR does
Per "reexport what is normal documented use, but not whole dependencies", it does not restore the blanket
@reexport. It adds an explicitexportlist, matching the Sundials.jl #553 pattern:bandsargumentBandedMatrix,brand,brandnBand,BandRange,band,bandrangebandwidth,bandwidths,colrange,rowrangeBandErrorDeliberately not restored, because they are BandedMatrices' own reexports or unrelated to this package: the FillArrays names
Fill,Ones,Zeros,Eye; theBandedMatricesmodule binding itself;symrcm. (The pre-strip surface was all ofnames(BandedMatrices)— 18 names plus everything FillArrays contributes; this is 12.)Evidence used to pick the list
AlmostBandedMatrix(bands::BandedMatrix, fill)is the documented constructor; every example insrc/,README.mdanddocs/src/index.mdbuildsbandswithbrand.brandnis its direct sibling for the same argument.test/imports added by the stripping commit — direct evidence the names used to come from the reexport:test/core_tests.jlusesBandedMatrix,brand,band(0), and@test_throws BandErroron this package's type;test/qa/alloc_tests.jlusesbrandand got ausing BandedMatricesadded — even thoughBandedMatricesis not a dependency oftest/qa/Project.toml. That import is dropped again here, which incidentally removes a latent failure in the QA lane.bandwidth/bandwidthsare the two names the stripping commit itself kept importing, and are how a user queriesbandpart(A).Also in this PR
brand(...)/BandedMatrixform; theBandedMatrices.qualifications inside the implementation are dropped too, so the bareimport BandedMatricesis gone (the non-public_banded_qr!/banded_qr_lmul!import is untouched).README.mdanddocs/src/api.mdnow document the reexported surface explicitly, including what is not reexported.test/qa/qa.jl:REEXPORTED_APIrestored and passed asreexports_allow, kept in sync with theexportblock.BandandBandErrorcarry no docstring upstream in BandedMatrices, so they are listed inapi_docs_kwargs = (; ignore = ...); everything else passes the public-API docstring check on its upstream docstring.names(FastAlmostBandedMatrices), in scope from a bareusing FastAlmostBandedMatrices, and===the upstream binding; andFill/Ones/Zeros/Eye/symrcm/BandedMatricesare not exported. The allow-list cannot drift from the exports without a test failure.Semver
Restoring names that used to be exported is not breaking — it only adds bindings back. No version bump here; a separate release pass handles versions.
Verification actually run (Julia 1.11.8, macOS aarch64)
Pkg.test()(Core group): 76 pass, 2 broken, 0 failGROUP=QA Pkg.test():alloc_tests.jl11 pass,qa.jl18 pass, 2 broken, 0 fail (the 2 broken are the pre-existingaqua_broken = (:ambiguities,)andei_broken = (:no_implicit_imports,))Runic --checkclean over the whole repousing FastAlmostBandedMatricesalone now suffices:AlmostBandedMatrix(brand(...), ...),AlmostBandedMatrix(BandedMatrix(...), ...),A[band(0)] .+= 1:n,bandwidths(bandpart(A))Docs were not built locally.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Rmh6B9eoW3N8oCbuuVPVxJ